feat: per-task dead-letter list + purge#314
Merged
Merged
Conversation
list_dead_by_task / purge_dead_by_task across SQLite/Postgres/Redis. Diesel filters by task_name; Redis scans dlq:all and filters (no secondary index).
Queue.listDeadByTask / purgeDeadByTask over the new core ops, with an in-memory backend impl. Live-verified by DeadLetterByTaskTest.
Queue.deadLettersByTask / purgeDeadByTask over the new core ops.
|
Warning Review limit reached
More reviews will be available in 43 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughTask-scoped dead-letter listing and purge APIs are added to core storage and exposed through Java, Node, and Python bindings. The new list methods support pagination and the purge methods return removed counts. ChangesTask-scoped dead-letter APIs
Sequence Diagram(s)sequenceDiagram
participant DefaultQueue
participant JniQueueBackend
participant NativeQueue
participant StorageBackend
participant RedisStorage
DefaultQueue->>JniQueueBackend: listDeadByTask(taskName, limit, offset)
JniQueueBackend->>NativeQueue: listDeadByTaskJson(handle, taskName, limit, offset)
NativeQueue->>StorageBackend: list_dead_by_task(task_name, limit, offset)
StorageBackend->>RedisStorage: list_dead_by_task(task_name, limit, offset)
RedisStorage-->>StorageBackend: Vec<DeadJob>
StorageBackend-->>NativeQueue: Vec<DeadJob>
NativeQueue-->>JniQueueBackend: JSON string
JniQueueBackend-->>DefaultQueue: JSON string
DefaultQueue->>DefaultQueue: decode DeadJob[]
DefaultQueue->>JniQueueBackend: purgeDeadByTask(taskName)
JniQueueBackend->>NativeQueue: purgeDeadByTask(handle, taskName)
NativeQueue->>StorageBackend: purge_dead_by_task(task_name)
StorageBackend->>RedisStorage: purge_dead_by_task(task_name)
RedisStorage-->>StorageBackend: deleted count
StorageBackend-->>NativeQueue: deleted count
NativeQueue-->>JniQueueBackend: jlong count
JniQueueBackend-->>DefaultQueue: long count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/taskito-core/src/storage/diesel_common/dead_letter.rs`:
- Around line 97-110: Normalize pagination inputs in list_dead_by_task so the
SQL backend matches Redis behavior: treat any limit <= 0 as returning an empty
result immediately, and clamp any negative offset to 0 before building the
dead_letter query. Apply this in the dead_letter.rs implementation around
list_dead_by_task, using the existing dead_letter::table query path and
DeadLetterRow::as_select() flow, so callers get consistent backend parity
regardless of signed inputs.
In `@crates/taskito-core/src/storage/redis_backend/dead_letter.rs`:
- Around line 174-191: The pagination target in the dead-letter scan can
overflow because `offset` and `limit` are cast from public `i64` inputs before
being added in the `dead_letter` lookup logic. Update the match-limit check in
this path to use a saturating or checked addition for the page target, and keep
the early-break behavior in the `DeadJob`/`DeadJobEntry` scan unchanged
otherwise.
- Around line 213-219: The DLQ purge logic in dead_letter.rs currently ignores
records that fail deserialization in the entry scan and still reports success,
which can hide skipped task-owned entries. Update the purge flow around the
DeadJobEntry parsing path so unreadable records are treated as an error or
otherwise surfaced instead of silently continuing, and make sure the returned
removed count only reflects entries that were actually verified and deleted in
the purge routine.
In
`@sdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.java`:
- Around line 266-270: The purgeDeadByTask method in InMemoryQueueBackend is not
synchronized with the DLQ write path, so its before - dead.size() count can race
with onFail(...) appending to dead. Make the remove-and-count operation atomic
by performing the purge while holding the same backend monitor used by the other
dead-queue mutations, and compute the removed count from that synchronized
section so matching entries added concurrently cannot slip through.
In `@sdks/node/test/core/inspection.test.ts`:
- Around line 79-82: The async polling in inspection.test.ts is too aggressive
and can flake while waiting for the worker to start, fail three jobs, and
persist DLQ rows. Increase the wait budget in the deadLetters check or switch
this block to the same eventual-wait helper used by the other async tests,
keeping the logic around queue.deadLetters() and the polling loop intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c5e4374-e221-44d7-aaa0-6149e070d7d2
📒 Files selected for processing (18)
crates/taskito-core/src/storage/diesel_common/dead_letter.rscrates/taskito-core/src/storage/mod.rscrates/taskito-core/src/storage/redis_backend/dead_letter.rscrates/taskito-core/src/storage/traits.rscrates/taskito-core/tests/rust/storage_tests.rscrates/taskito-java/src/queue/admin.rscrates/taskito-node/src/queue/admin.rscrates/taskito-python/src/py_queue/inspection.rssdks/java/src/main/java/org/byteveda/taskito/DefaultQueue.javasdks/java/src/main/java/org/byteveda/taskito/Queue.javasdks/java/src/main/java/org/byteveda/taskito/internal/JniQueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/internal/NativeQueue.javasdks/java/src/main/java/org/byteveda/taskito/spi/QueueBackend.javasdks/java/src/test/java/org/byteveda/taskito/DeadLetterByTaskTest.javasdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.javasdks/node/src/queue.tssdks/node/test/core/inspection.test.tssdks/python/taskito/_taskito.pyi
Diesel now clamps limit/offset like Redis (non-positive limit = empty page); Redis purge propagates a deserialize error instead of silently skipping (which under-reported the count); saturating page target avoids i64->usize overflow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Task-scoped dead-letter management across all four SDKs: list / purge DLQ entries for a single task. The existing DLQ surface only worked by id or over the whole queue.
Why a core op (not a client-side filter)
list_deadis paginated (limit/offset) over the entire DLQ, so filtering by task in the client breaks pagination (you filter one page at a time). Correct paging needs server-side filtering — so two ops are added once in the core and shared by every SDK.Layers
list_dead_by_task(task, limit, offset)+purge_dead_by_task(task)on theStoragetrait + SQLite/Postgres/Redis + delegate. Diesel filters bytask_name; Redis scansdlq:alland filters in memory (no secondary index — matches the periodic fix).dead_letters_by_task/purge_dead_by_task+ stubs.Queue.listDeadByTask/purgeDeadByTask+ in-memorytest-supportimpl.Queue.deadLettersByTask/purgeDeadByTask.Verification
test_dead_letter_by_task— green on SQLite + Redis (live, local docker); Postgres compiles + runs in CI. Covers list, pagination, purge, and that another task's entries survive.DeadLetterByTaskTest(JNI-backed, fail→DLQ→query/purge) and a Nodeinspection.test.tscase — both live.cargo checkdefault + postgres + redis;cargo clippyclean; full./gradlew buildgreen; Node 185 tests + typecheck + biome clean; Pythonruff+mypyclean.This completes the P14 scope (retry backoff #311, periodic CRUD #313, per-task DLQ here).
Summary by CodeRabbit
New Features
Tests